Only apply MCMT plugin on MCMTGate
#13596
Conversation
Cryoris
added
Changelog: Bugfix
Cryoris
requested review from
alexanderivrii,
ShellyGarion and
a team
as code owners
|
One or more of the following people are relevant to this code:
|
Pull Request Test Coverage Report for Build 12649042437Warning: This coverage report may be inaccurate.This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.
Details
💛 - Coveralls |
| mcmt = MCMT(gate=gate, num_ctrl_qubits=1, num_target_qubits=1) | ||
| circuit.append(mcmt, circuit.qubits) # append the MCMT circuit as gate called "MCMT" |
There was a problem hiding this comment.
A bit off-topic. Here we are appending the quantum circuit mcmt to the quantum circuit circuit, yet the docstring for append states instruction: Operation | CircuitInstruction. The code does work, since a QuantumCircuit defines a to_instruction method, which append tries to use when the instruction is not of type Operation. Would it be slightly cleaner to change to circuit.append(mcmt.to_instruction(), ...), or should we change the typehint for the append method?
There was a problem hiding this comment.
Updating the docs for append sounds good, since we use append(QuantumCircuit) all over the place 🤔
There was a problem hiding this comment.
Thanks @Cryoris for also updating our included plugins so that they all follow the same convention. And you have taken the art of writing tests to a completely new level -- now they read like a fiction story (unfortunate names, intruder circuits, etc) 😄.
|
Haha I'm hoping to provide some joy for people reviewing the code 😛 |
|
@Mergifyio backport stable/1.3 |
✅ Backports have been created
|
* Only apply MCMT plugin on MCMTGate * add checks for other gates * fix reno * use == instead of equiv (cherry picked from commit 0eeba0d) Co-authored-by: Julien Gacon <[email protected]>
Summary
Fixes #13563, by enabling the high-level synthesis plugins of MCMT only for the
MCMTGate, not any gate called"mcmt".Details and comments
While we often let it be the user's responsibility to be mindful with gate names, this is a special case as we provide a circuit called
"mcmt". This can cause innocent-looking code to break, so we should add this check at least until theMCMTcircuit is removed 🙂